fix(podman): harden sandbox callbacks by maxamillion · Pull Request #2111 · NVIDIA/OpenShell

maxamillion · 2026-07-01T21:17:34Z

Summary

Improve Podman diagnostics and preflight behavior when the OpenShell gateway runs inside a container. Containerized gateways now report actionable guidance for rootless Podman socket permission failures and avoid misleading subordinate-ID warnings based on the container's filesystem rather than the Podman host.

This PR no longer changes callback listener discovery or gateway network exposure; that design work is being handled separately.

Related Issue

Refs #1909

Changes

Detect when the gateway process is running inside a container.
Add --userns=keep-id guidance when a containerized gateway cannot access a mounted rootless Podman socket due to Permission denied.
Apply the enriched diagnostic to both Podman socket retry warnings and the final connection error.
Skip the local /etc/subuid and /etc/subgid preflight inside gateway containers, where those files do not describe the host running the Podman service.
Preserve existing diagnostics and preflight behavior for direct-host deployments, root, macOS, and unrelated connection errors.
Add focused unit coverage for the new diagnostic and preflight decisions.

Testing

mise run pre-commit passes (not run locally because mise is unavailable in the current environment)
cargo test -p openshell-driver-podman --lib
cargo fmt --all -- --check
cargo clippy -p openshell-driver-podman --all-targets -- -D warnings
Unit tests added/updated
E2E tests not applicable; this change only affects startup diagnostics and a host preflight warning

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
Architecture docs not applicable; no runtime boundaries, callback behavior, or configuration changed

copy-pr-bot · 2026-07-01T21:17:38Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

maxamillion · 2026-07-02T19:14:49Z

/ok to test 74eb820

elezar · 2026-07-03T13:13:12Z

 const VOLUME_PREFIX: &str = "openshell-sandbox-";

+/// Secret name prefix for per-sandbox gateway JWTs.
+const TOKEN_SECRET_PREFIX: &str = "openshell-token-";


Do secret names allow "namespacing"? (i.e. would something like token.openshell/ be valid)?

elezar · 2026-07-03T13:18:20Z

+    pub gateway_tls_enabled: bool,
+    /// Whether gateway callbacks are protected by an auth mechanism.
+    #[serde(skip)]
+    pub gateway_callback_auth_enabled: bool,


nit: I would assume that these gateway-specific properties are shared across other drivers. Does it make sense to embed them in a struct?

elezar · 2026-07-03T13:23:25Z

+                uid: 0,
+                gid: 0,


UID=0 and GID=0 are the defaults. Also, does this need to be readable by any other users? (we only specified ro when this was handled as a mount).

elezar · 2026-07-03T13:31:26Z

@maxamillion, is the use of Podman secrets separate from the callback changes? If so, could we pull the secret changes into a separate PR? They independently fix the containerized gateway token-path problem, while the callback changes affect broader behavior—for example, automatically selecting a host interface and exposing the full gateway service on the additional listener. Separating them would make both changes easier to review and test.

maxamillion · 2026-07-03T13:48:58Z

@elezar yes, but that change is also related to fixes for issue ticket I referenced so I wasn't sure if I should break it out or leave them combined. I'm happy to split them if preferred.

elezar · 2026-07-03T16:42:39Z

@elezar yes, but that change is also related to fixes for issue ticket I referenced so I wasn't sure if I should break it out or leave them combined. I'm happy to split them if preferred.

I definitely have some thoughts on the callback changes, but don't want to block the secret changes on those, so splitting them out may be cleaner.

maxamillion · 2026-07-06T22:33:38Z

@elezar I split the secrets out and updated this PR #2156

elezar · 2026-07-10T15:35:36Z

I think the callback-listener portion needs further design work before this can merge.

It adds another in-process, driver-specific gateway hook mirroring Docker’s gateway_bind_addresses(): gateway bind and TLS state are injected into PodmanComputeConfig, and the concrete Podman driver determines an additional gateway listener. This cuts across the driver-alignment concerns discussed in #1952 and #2061 and has no equivalent for external drivers.

The containerized path also skips listener discovery while the documented deployment still publishes only to host loopback, so that part of #1909 remains unresolved.

Could we split out the independent diagnostics and preflight fixes? I’m working on the broader callback-listener design separately, so I’m not expecting you to resolve that design question in this PR.

Skip host subordinate-ID checks inside gateway containers and add actionable keep-id guidance for mounted rootless Podman socket permission failures.\n\nRefs NVIDIA#1909 Signed-off-by: Adam Miller <admiller@redhat.com>

maxamillion · 2026-07-14T20:45:41Z

@elezar I've limited this now to diagnostic info and preflight behavior, I'll work on a different PR that addresses the callback listener, interface binding, and RPM changes to properly resolve the remaining issues in #1909

russellb · 2026-07-14T20:50:31Z

Agent-assisted code review. Reviewed at head 87005b3.

Thanks for tightening the scope and fixing the body — it matches the diff now. One blocker and a couple of follow-ups.

Blocking

PR title is stale. It still reads fix(podman): harden sandbox callbacks, but this diff has no callback/network/TLS changes. Squash-merge uses the PR title as the commit subject, so this inaccurate line would land on main. Suggest: fix(podman): diagnostics for containerized rootless socket + subuid preflight.

Should fix

Doc gap. crates/openshell-driver-podman/README.md:353-357 lists the check_subuid_range() skip conditions (macOS/root) but not the new container skip or the --userns=keep-id guidance. Per AGENTS.md, update the crate README for this user-facing behavior change.
#1909 scope. This fully closes only the subuid false-positive and adds guidance (not a fix) for keep-id. Refs #1909 is correct — please keep it (not Fixes). A body line noting which root cause is actually resolved would help.

Nit

enrich_socket_connection_error matches on the "Permission denied" substring. That's the right call here (the io::Error kind is already flattened to a String in client.rs:281, so ErrorKind::PermissionDenied isn't available without threading it through PodmanApiError). Worth a one-line comment so it isn't "fixed" naively later.

maxamillion requested review from a team, derekwaynecarr and mrunalp as code owners July 1, 2026 21:17

elezar reviewed Jul 3, 2026

View reviewed changes

maxamillion force-pushed the gateway_callback branch from 74eb820 to 411fc1f Compare July 6, 2026 22:31

maxamillion mentioned this pull request Jul 6, 2026

fix(podman): deliver sandbox JWTs as secrets #2156

Merged

8 tasks

elezar mentioned this pull request Jul 10, 2026

refactor(compute): define a uniform gateway callback endpoint contract #2215

Open

maxamillion force-pushed the gateway_callback branch from 411fc1f to f6c705e Compare July 14, 2026 20:41

fix(podman): improve container diagnostics

87005b3

Skip host subordinate-ID checks inside gateway containers and add actionable keep-id guidance for mounted rootless Podman socket permission failures.\n\nRefs NVIDIA#1909 Signed-off-by: Adam Miller <admiller@redhat.com>

maxamillion force-pushed the gateway_callback branch from f6c705e to 87005b3 Compare July 14, 2026 20:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(podman): harden sandbox callbacks#2111

fix(podman): harden sandbox callbacks#2111
maxamillion wants to merge 1 commit into
NVIDIA:mainfrom
maxamillion:gateway_callback

maxamillion commented Jul 1, 2026 •

edited

Loading

Uh oh!

copy-pr-bot Bot commented Jul 1, 2026

Uh oh!

maxamillion commented Jul 2, 2026

Uh oh!

elezar Jul 3, 2026

Uh oh!

elezar Jul 3, 2026

Uh oh!

elezar Jul 3, 2026

Uh oh!

elezar commented Jul 3, 2026 •

edited

Loading

Uh oh!

maxamillion commented Jul 3, 2026

Uh oh!

elezar commented Jul 3, 2026

Uh oh!

maxamillion commented Jul 6, 2026

Uh oh!

elezar commented Jul 10, 2026

Uh oh!

maxamillion commented Jul 14, 2026

Uh oh!

russellb commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

maxamillion commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Testing

Checklist

Uh oh!

copy-pr-bot Bot commented Jul 1, 2026

Uh oh!

maxamillion commented Jul 2, 2026

Uh oh!

elezar Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

elezar Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

elezar Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

elezar commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maxamillion commented Jul 3, 2026

Uh oh!

elezar commented Jul 3, 2026

Uh oh!

maxamillion commented Jul 6, 2026

Uh oh!

elezar commented Jul 10, 2026

Uh oh!

maxamillion commented Jul 14, 2026

Uh oh!

russellb commented Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

maxamillion commented Jul 1, 2026 •

edited

Loading

elezar commented Jul 3, 2026 •

edited

Loading